Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid a useless redirect with site title and home link blocks #65672

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

Chouby
Copy link
Contributor

@Chouby Chouby commented Sep 26, 2024

What?

Fixes #65670

Why?

Avoid a redirect caused by the link in the site title block when WordPress is installed in a sufolder or on multisite with sites in folder.

How?

Use home_url( '/' ) instead of home_url()`

Testing Instructions

  1. Install WordPress in a subfolder
  2. Create a page and insert the site title block
  3. View the page on frontend
  4. Open the browser tools to observe that there will be no redirect.
  5. Check the link proposed by the site title block is https://mysite.com/wordpress/ instead of https://mysite.com/wordpress in the current stable version.
  6. Click on the link and observe that there is no 301 redirect

Copy link

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Chouby <[email protected]>
Co-authored-by: Mamaduka <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Block] Site Title Affects the Site Title Block labels Sep 26, 2024
@Chouby Chouby changed the title Avoid a useless redirect with the site block title Avoid a useless redirect with site title and home link blocks Sep 26, 2024
@Mamaduka Mamaduka added the [Block] Home Link Affects the Home Link Block label Sep 26, 2024
@Mamaduka Mamaduka self-requested a review September 26, 2024 14:01
@Mamaduka
Copy link
Member

@Chouby, do you mind rebasing your branch on top of the latest trunk? That should unblock GitHub CI checks.

See: https://developer.wordpress.org/block-editor/contributors/code/git-workflow/#keeping-your-branch-up-to-date

@Chouby
Copy link
Contributor Author

Chouby commented Sep 29, 2024

One test is still failing :

$expected = '<li class="wp-block-navigation-item"><h1 class="wp-block-site-title"><a href="http://' . WP_TESTS_DOMAIN . '" target="_self" rel="home">Test Blog</a></h1></li>';

It's not very surprising as 'http://' . WP_TESTS_DOMAIN doesn't account the fact that the home url can sometimes have a trailing slash and sometimes not.
My first idea to work around this would be to replace it by home_url( '/' ) but I usually don't like to use the same function in the test and the tested code. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Home Link Affects the Home Link Block [Block] Site Title Affects the Site Title Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Site title block links to wrong home url causing a useless redirect
2 participants